-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Add session to agents #4216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add session to agents #4216
Conversation
|
I think right direction - assuming session id == a new agent, ie open a session, get an agent created and assigned. This is the same as CLI effectively, but without it being at the electron window level (which was also the same effectively as the cli). I assume would be some old code to cleanout like |
yeah that bit is still being worked on. new agent -> goosed gives you a session id and that's what you pass around for state, almost nothing else. continue on session, same but you don't need to create it |
Co-authored-by: Douwe Osinga <[email protected]>
| .get_agent() | ||
| .await | ||
| .map_err(|_| StatusCode::PRECONDITION_FAILED)?; | ||
| let agent = state.get_agent().await; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need that PRECONDITION_FAILED? anymore? (not sure what we would do with it anyway)
| request.messages.len() | ||
| ); | ||
|
|
||
| let error_response = CreateRecipeResponse { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, so no need for all these now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, Agent is no longer an Option<> on AppState, so these errors are gone
|
ouch yeah this is a huge one, some tyre kicking and things seem to work, but there is a bit touched with recipes not really clear how to test/validate (and more than enough code to review). Some potentially nasty conflicts that need to be resolved unfortunately, but seems to work. |
| self.agent | ||
| .clone() | ||
| .ok_or_else(|| anyhow::anyhow!("Agent needs to be created first.")) | ||
| pub async fn get_agent(&self) -> AgentRef { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does AppState simply track one agent? Is the plan to refactor AppState to be able to retrieve from multiple agents get_agent(session_id)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes exactly, the plan following this change is to make agent no longer a singleton in goose-server.
| ) -> Result<Json<StartAgentResponse>, StatusCode> { | ||
| verify_secret_key(&headers, &state)?; | ||
|
|
||
| state.reset().await; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would expect to append the newly started agent to the state rather then resetting/overriding it?
Yeah it's got a broad surface area, but it's a necessary step to get to a point where we can have one backend serving multiple sessions for e.g. tabbed conversations in the UI. It also does a bit more in that it moves some recipe handling to the backend when it previously was all in the UI. |
* main: Align Dynamic Task Interface with Recipe Interface (#4311) docs: copilot auth and mcp-ui links (#4497) docs: July and August 2025 Community All-Stars Update (#4501) remove clicking outside to close recipe warning (#4502) lower min width to 450 for small screens Convert recipe create and import forms to use tanstack form and zod schema validation (#4499) Repo CI: use a writable location for Goose home directory (#4500) feat: Add functionality to delete session in history list view (#4480) fix: recipe deeplink "+" characters and folder change (#4471) Add session to agents (#4216) fix: need to send errors to appropriate stream (#4491) Add Docker support for Goose in CI/CD pipelines (#4434) Add visual indicator while recipe loads (#4447) Disable chat input while extensions load (#4417) chore(release): release version 1.7.0 (#4391) fix double filtering (#4409) Rewrite the developer mcp using the rmcp sdk (#4297) docs: sessions reorg and conversation features (#4462)
* 'main' of github.com:block/goose: Align Dynamic Task Interface with Recipe Interface (#4311) docs: copilot auth and mcp-ui links (#4497) docs: July and August 2025 Community All-Stars Update (#4501) remove clicking outside to close recipe warning (#4502) lower min width to 450 for small screens Convert recipe create and import forms to use tanstack form and zod schema validation (#4499) Repo CI: use a writable location for Goose home directory (#4500) feat: Add functionality to delete session in history list view (#4480) fix: recipe deeplink "+" characters and folder change (#4471) Add session to agents (#4216) fix: need to send errors to appropriate stream (#4491) Add Docker support for Goose in CI/CD pipelines (#4434) Add visual indicator while recipe loads (#4447) Disable chat input while extensions load (#4417) chore(release): release version 1.7.0 (#4391)
Co-authored-by: Douwe Osinga <[email protected]> Co-authored-by: Jack Amadeo <[email protected]> Co-authored-by: Jack Amadeo <[email protected]> Signed-off-by: Matt Donovan <[email protected]>
Co-authored-by: Douwe Osinga <[email protected]> Co-authored-by: Jack Amadeo <[email protected]> Co-authored-by: Jack Amadeo <[email protected]> Signed-off-by: HikaruEgashira <[email protected]>
Add session id to everything and pass those on to the agent